Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add full help screen to help plugin #714

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

giflw
Copy link
Contributor

@giflw giflw commented Jan 15, 2019

All actual help still appears on short help.
Use parameter short: true to show message on short and full help.

All actual help still appears on short help.
Use parameter `short: true` to show message on short and full help.
Copy link
Member

@henrikingo henrikingo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here too I've outlined a different approach more in line with how impress.js was designed to do this.

*
* <!-- Show a help popup at start, or if user presses "H" -->
* <div id="impress-help"></div>
*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to stay. Plugins that display something on the screen are required to be optional. One way to make them optional is that the presentation author must explicitly add an empty div for them to appear.

In the case of the help plugin, note that it is shown automatically when the presentation is loaded. Due to this, the div must stay.

color: "#FFF",
opacity: 0.8,
textAlign: "center",
padding: "3em"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The philosophy of impress.js is that we don't want to force any colors or other styling for the user. So instead of setting the style here, you should set it in css/impress-demo.css. Unfortunately this means you also need to set this in every other css file under presentations. (Related: #681)

So what is left for the plugin to do? Maybe nothing? Do you need different CSS for the full help?

At most, you could set a class that indicates short help vs full help:

<div id="impress-help" class="help-short">
<div id="impress-help" class="help-full">

Presentation authors can then choose to use this class for styling if they want.

@janishutz
Copy link
Contributor

I don't think this is something that needs a plugin update. I would much rather have it in a impress-extra.css file. I also think that we should have like a impress.css file where important and common CSS is defined, which may or may not be included

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants